Skip to content
This repository was archived by the owner on Mar 18, 2025. It is now read-only.

Cargo workspace yassin feedback #136

Merged
merged 8 commits into from
Nov 19, 2023
Merged

Cargo workspace yassin feedback #136

merged 8 commits into from
Nov 19, 2023

Conversation

@YassinEldeeb
Copy link
Contributor

@YassinEldeeb YassinEldeeb commented Nov 19, 2023 •
edited by dotansimha
Loading

What is applied from @dotansimha's PR?

  • Resolve Actix-web Tail segments must have names. warning
  • Remove strip = false from CI, since it's defaulted to false
  • Rename async_runtime crate to wasm_polyfills
  • Use wasm_polyfills items inline without imports for better clarity.
  • Separate crates/* into bin/* and libs/*
  • Rename from_yaml to parse_config_from_yaml for better clarity, and extract parse_config_from_json as a utility as well + reuse those in the main flow.
  • Remove the unused openssl dependency

@YassinEldeeb YassinEldeeb mentioned this pull request Nov 19, 2023
Merged
11 tasks
@YassinEldeeb
Copy link
Contributor Author

YassinEldeeb commented Nov 19, 2023

Agh...rebasing files I never touched is a pain...🥲

@dotansimha
Copy link
Member

dotansimha commented Nov 19, 2023

@YassinEldeeb why not target my branch? why is it targeted to master?

@dotansimha dotansimha changed the base branch from master to cargo-workspace November 19, 2023 07:05
@dotansimha
Copy link
Member

dotansimha commented Nov 19, 2023 •
edited
Loading

  • Without openssl, the compilation inside Docker image (as part of the Dockerize step) fails...
  • Fix CI and paths for Docker build
  • Fix WASM CI with new paths

@github-actions
Copy link

github-actions bot commented Nov 19, 2023 •
edited
Loading

🐋 This PR was built and pushed to the following Docker images:

Docker Bake metadata 
{
"conductor": {
  "containerimage.config.digest": "sha256:8e686644095ed924e63ef55cc02309142af9236b56f1823ecd771ed9302134fd",
  "containerimage.descriptor": {
    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
    "digest": "sha256:2dda9bea5d1cdd50cdd10624b07a5fc107e9d09dac1d27a8afd74fcd2ebebca6",
    "size": 901,
    "platform": {
      "architecture": "amd64",
      "os": "linux"
    }
  },
  "containerimage.digest": "sha256:2dda9bea5d1cdd50cdd10624b07a5fc107e9d09dac1d27a8afd74fcd2ebebca6",
  "image.name": "ghcr.io/the-guild-org/conductor-t2/conductor:a365171d1bafed7abff120040b7f78cb900d1a47"
}
}

@dotansimha dotansimha merged commit d3ac18f into cargo-workspace Nov 19, 2023
@dotansimha dotansimha deleted the cargo-workspace-yassin-feedback branch November 19, 2023 08:58
dotansimha added a commit that referenced this pull request Nov 19, 2023
@YassinEldeeb @dotansimha
Cargo workspace yassin feedback (#136)
20a0117 
Co-authored-by: Dotan Simha <[email protected]>
dotansimha added a commit that referenced this pull request Nov 19, 2023
@dotansimha @YassinEldeeb
Cargo workspace yassin feedback (source: #136) (#141)
1af650f 
Co-authored-by: Yassin Eldeeb <[email protected]>
@YassinEldeeb
Copy link
Contributor Author

YassinEldeeb commented Nov 19, 2023

@YassinEldeeb why not target my branch? why is it targeted to master?

oh, that's the reason...I missed it 😄

Thanks for all the work on landing it!! 🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@dotansimha dotansimha dotansimha approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@YassinEldeeb @dotansimha